Repository navigation
fix(display): schedule windows end at the end time; on-demand ending in off hours blanks at once - #714
Conversation
…in off hours blanks at once Two run-loop bugs recorded by #704's schedule golden trace (docs/RUN_LOOP_REDESIGN.md items 5 and 6). - _in_window() is half-open, [start, end). The closed end counted the end minute as on, but only for a check at exactly hh:mm:00.000, which the once-a-minute gate then held; so the panel went off at the start or the end of that minute depending on timing. Applies to the dim schedule too, which shares the helper. Midnight-crossing windows: now >= start or now < end. - _reset_on_demand_fields() clears the schedule minute gate. On-demand forces is_display_active on over a scheduled-off answer; when the session expired or was stopped nothing recomputed it until the next clock minute, so rotation kept showing for up to 60 s in off hours. Golden: only schedule.json changes. Off at t=90 (23:01:00, the end time) instead of t=150, and blank at t=190 (the on-demand expiry) instead of t=210. All other goldens are byte-identical. Docs: CONFIG_REFERENCE spells out [start, end) for schedule and dim_schedule; ARCHITECTURE notes it; RUN_LOOP_REDESIGN items 5-6 removed; CHANGELOG Unreleased -> Fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rt, end) Both tests mean "off at 23:00:00" and wrote end_time 22:59, which only worked because the end minute used to count as on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
…lds to it (#712) A WiFi notice preempts the current screen within about a second (frame loops, post-loop check, make-up dwell), and an interrupted Vegas iteration that yielded for a notice ends the pass so the notice shows next. Goldens: wifi_notice, vegas. First of three run-loop fixes (#712, #713, #714), pre-tested together. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CHANGELOG keeps all Fixes entries. RUN_LOOP_REDESIGN.md: with #712 and #713 in, removing items 5 and 6 empties the 'may be wrong' list, so the section now says all six are fixed and by which PR. Goldens regenerated: byte-identical to the merged files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Post-merge rig check on hdpi: SKIPPED (not run) The run-loop checks were queued after the #702 soak, on origin/main 41db488, which contains #712, #713 and #714. They never ran. At 20:07 EDT another session on hdpi made these changes:
My guard stopped the batch at that point. Their config was left as they set it, and HEAD was put back to 4ddc3a3. Nothing about this PR was measured. For a re-run:
🤖 Generated with Claude Code |
|
ledpi check (Pi 4, 192x48), main 746dcfc: PASS.
Observation (probably not from this PR):
🤖 Generated with Claude Code |
Fixes items 5 and 6 of "Behaviour the traces pin down that may be wrong" in
docs/RUN_LOOP_REDESIGN.md, both recorded by #704'sschedulegolden trace.Root causes
1. On-demand ending in scheduled-off hours left the panel on until the next minute.
_evaluate_schedule()lets an on-demand session override the schedule by settingis_display_active = Trueon top of_check_schedule()'s answer. When the session ends (expired or stopped),_clear_on_demand()->_reset_on_demand_fields()drops the override flag, butis_display_activestaysTrue:_check_schedule()is gated to once per clock minute (_schedule_checked_minute) and had already run that minute, so nothing recomputed it until the minute changed. Up to 60 s of rotation in off hours.2. A window's end minute counted as on, and only sometimes.
_in_window()was closed,start <= now <= end, andnowcarries seconds. So at the end minute only a check landing on exactlyhh:mm:00.000saw "on"; the minute gate then held that answer for the whole minute. A check at any later second saw "off". The panel went off at 23:00 or 23:01 depending on when the minute's first check ran. (The golden harness's fake clock hits the boundary exactly, which is why the trace showed the late case.)Fix (
src/display_controller.py, two hunks)_in_window()is half-open,[start, end):start <= now < end, and for windows that cross midnightnow >= start or now < end. On from the start time, off at exactly the end time, whatever second the check runs._reset_on_demand_fields()clears_schedule_checked_minute, so the next_evaluate_schedule()(the next_service_pending_changes()pass, at most 0.25 s later, or the next loop pass) recomputes the real schedule answer and_blank_while_scheduled_off()takes over. Both expiry and stop go through this path.The dim schedule shares
_in_window(), so it gets the same[start, end)rule. It did not need the second fix: on-demand never overrides the dim answer.start == endis an empty window (never on). Before, it was on only for a check at exactlyhh:mm:00.000, so effectively the same.End-time semantics and docs
Owner direction was half-open
[start, end). The web UI's schedule picker (schedule-picker.js,partials/schedule.html) only labels the fields "Start Time" / "End Time" and makes no claim either way.docs/CONFIG_REFERENCE.mdcalled them "on/off times", which matches. Docs touched:docs/CONFIG_REFERENCE.md:schedulenow spells out "on fromstart_timeup to, but not including,end_time", midnight crossing, per-day ("the entry for the current day decides"), and on-demand ending blanking within about a second.dim_schedulesays it has the same boundaries.docs/ARCHITECTURE.md: the schedule bullet notes the half-open windows and the immediate re-check when on-demand ends.docs/RUN_LOOP_REDESIGN.md: removed items 5 and 6. Items 1-4 are untouched for the parallel wifi/live-takeover PRs.CHANGELOG.md: two entries under Unreleased -> Fixes.Golden trace diff
Only
schedule.jsonchanged. The other 14 goldens are byte-identical: the whole golden file passes with only this fixture regenerated (LEDMATRIX_REGEN_GOLDEN=1 pytest test/test_run_loop_golden.py -k schedule). The harness clock starts at 22:59:30. The schedule is23:05-23:01(off from 23:01 to 23:05) and dim is23:00-23:01. On-demandweatherstarts at t=170 for 20 s. So t=90 is 23:01:00 (the end minute), t=190 is 23:02:40 (expiry) and t=330 is 23:05:00.Row by row:
[80, clock]: was a full 20 sdurationscreen. Now cut at t=90 (23:01:00) withschedule-off: the end time turns the panel off. 11 frames = t=80-90 at 1 fps.[100, weather],[120, clock],[140, weather … schedule-off]: removed. These were the whole 23:01 end minute being shown, because the check at exactly 23:01:00.000 counted it as "on" and the minute gate kept that until 23:02:00 (t=150).[150, <off>, 20 …]->[90, <off>, 80 …]: the off stretch now starts at the end time and runs until the on-demand request at t=170. The frame count is blank pushes: one per 60 s off pass (t=90, 150), plus the repaint at t=170 just before on-demand takes over, which the old row also had (t=150, 170).[170, weather … on-demand-expired]: unchanged.[190, weather, 20, schedule-off]: removed. After expiry at 23:02:40 the rotation showedweatheruntil the 23:03:00 minute tick, the first bug.[210, <off>, 120 …]->[190, <off>, 140 …]: the panel blanks at the expiry itself. Blank pushes at t=190, 250, 310, one per 60 s pass (was t=210, 270).schedule-off150 -> 90 and 210 -> 190: the same two transitions, at the right times.Tests
test/test_display_controller_schedule.py:TestScheduleWindows:test_boundaries_are_inclusivebecomestest_window_is_half_open(08:59:59 off, 09:00 on, 16:59:59 on, 17:00 off). The midnight test now asserts 20:59:59 off, 21:00 on, 00:00 on, 06:59:59 on, 07:00 off.TestEndMinuteBoundary: for a same-day window, a midnight-crossing window and a per-day midnight-crossing window, the first check of the end minute at:00and at:59is off. The minute gate keeps that answer for the rest of the minute. The start minute is on at both:00and:59.TestOnDemandEndsDuringScheduledOff: the override keeps the panel on, then_reset_on_demand_fields()followed by_evaluate_schedule()in the same clock minute is off. Ending in on hours stays on.TestOnDemandEndsDuringScheduledOffRunLoop: runs the realrun()loop through refactor(display): run() stage 1 - golden traces and extracted helpers, no behaviour change #704's harness. An expiry at 23:02:35 blanks at t=185 exactly. A stop request at 23:02:31 blanks within one second.TestDimBoundaries: half-open same-day, midnight-crossing and per-day dim windows, plus a dim end minute whose first check is at:59.test/test_display_pending_changes.py: twoTestRunLoopBlanksWhenVegasHandsBacktests mean "off at 23:00:00" (off_at = t0 + 60) but wroteend_time: '22:59'. That only worked because the end minute counted as on. They now say'23:00'. These were the only new failures in the first full-suite comparison.Mutation-checked. Reverting the gate reset fails the 3 on-demand tests and the
schedulegolden. Reverting_in_windowto closed fails 11 tests and the golden.Verification
origin/mainin a separate worktree: the same 61 failed / 6 errors on both (the pre-existing Windows set: install_lowmem, pixlet download, starlark routes and ownership, and a few others), with no new and no fixed IDs. Branch: 7400 passed. The first branch run had 3 extra failures, thetest_display_pending_changes.pytests above; after the fix the ID sets match exactly.scripts/check_types.py(mypy 1.20.2, the CI pin): no issues in 92 modules.Not changed: in per-day mode a window crossing midnight is judged by the entry for the day being checked. Monday's 22:00-07:00 does not carry into Tuesday 03:00; Tuesday's entry decides. That is existing behaviour, documented now, not altered.
🤖 Generated with Claude Code